-
Notifications
You must be signed in to change notification settings - Fork 0
[CI] (3864784) next-js/15-app-router-todo #247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Now I have all the information needed to provide the evaluation. Let me compile the review: PR Evaluation ReportSummaryThis PR integrates PostHog analytics into a Next.js 15 App Router todo application. It adds both client-side (
Confidence score: 3/5 🤔
File changes
App sanity check: 3/5
|
| Criteria | Result | Description |
|---|---|---|
| App builds and runs | Yes | Build completes successfully with no errors |
| Preserves existing env vars & configs | No | Removed page-specific metadata from About page |
| No syntax or type errors | Yes | TypeScript compilation succeeds |
| Correct imports/exports | Yes | All PostHog imports are correct |
| Minimal, focused changes | Mostly | Changes are focused but include unnecessary metadata removal |
Issues
- Metadata regression: Converting About page to client component removed
metadataexport (title: 'About - Todo App',description: 'Learn more about this Next.js 15 todo application'). The comment claims it was "moved to layout.tsx" but no such file exists. Should either createapp/about/layout.tsxwith the metadata or usegenerateMetadata(). [MEDIUM] - No page view tracking: The integration captures custom events but doesn't explicitly set up page view tracking. While PostHog may auto-capture page views, it's not explicitly configured. [LOW]
Other completed criteria
- Build compiles successfully
- All imports resolve correctly
- No runtime errors in build output
- API routes function correctly
- Environment variables properly referenced via
process.env
PostHog implementation: 3/5 ⚠️
| Criteria | Result | Description |
|---|---|---|
| PostHog SDKs installed | Yes | posthog-js@^1.333.0 and posthog-node@^5.24.1 in package.json |
| PostHog client initialized | Yes | Client via instrumentation-client.ts, server via singleton in lib/posthog-server.ts |
| capture() | Yes | Events captured on both client and server for CRUD operations |
| Identify() | No | No posthog.identify() calls anywhere - users remain anonymous |
| Error tracking | Yes | capture_exceptions: true enabled in instrumentation-client.ts |
| Reverse proxy | Yes | Properly configured in next.config.ts with /ingest rewrites |
Issues
- Client-server identity linkage broken: Server-side events look for
x-posthog-distinct-idheader but client fetch requests never include this header. All server events will havedistinctId: 'anonymous', making it impossible to correlate client and server events for the same user. [CRITICAL] - No user identification: No
posthog.identify()call anywhere. While this app may not have authentication, the integration should demonstrate identification patterns or at least document this gap. [MEDIUM] - Server client uses NEXT_PUBLIC_POSTHOG_HOST: The server client uses
process.env.NEXT_PUBLIC_POSTHOG_HOSTwhich is set tohttps://us.i.posthog.com. This bypasses the reverse proxy (which uses/ingest). Should usehttps://us.i.posthog.comdirectly for server-side (which it does) but the variable naming is confusing. [LOW]
Other completed criteria
- API key accessed via environment variable (not hardcoded in code)
- Client uses reverse proxy (
api_host: "/ingest") - UI host correctly set to
https://us.posthog.com - Exception capture enabled
- Debug mode conditional on development environment
- Singleton pattern for server client prevents multiple instances
- Shutdown function provided for graceful cleanup
PostHog insights and events: 4/5 ✅
| Filename | PostHog events | Description |
|---|---|---|
todo-form.tsx |
todo_add_button_clicked |
Client-side click event with has_description property |
todo-item.tsx |
todo_toggle_clicked, todo_delete_button_clicked |
Client-side interaction events with todo_id, new_completed_state, was_completed properties |
todo-list.tsx |
about_page_link_clicked |
Navigation tracking |
app/about/page.tsx |
back_to_todos_clicked |
Navigation tracking |
api/todos/route.ts |
todo_created |
Server-side creation event with todo_id, has_description |
api/todos/[id]/route.ts |
todo_completed, todo_uncompleted, todo_deleted |
Server-side state change events with todo_id |
Issues
- Duplicate event tracking for same actions: Client captures
todo_toggle_clickedand server capturestodo_completed/todo_uncompletedfor the same user action. While this provides client vs server confirmation, it may inflate event counts and complicate analysis. Consider documenting the intent or consolidating. [LOW] - Missing error context in events: Failed API calls don't capture error events. Consider adding
todo_operation_failedevents for error tracking beyond exceptions. [LOW]
Other completed criteria
- Events represent real user actions (create, complete, delete, navigate)
- Events can support funnel analysis (add button → created, toggle → completed)
- Events enriched with relevant properties (todo_id, completion states, has_description)
- No PII captured in event properties
- Event names are descriptive and follow consistent naming convention
- Server events capture the "truth" while client events capture intent/timing
Reviewed by wizard workbench PR evaluator
Automated wizard CI run
Source: manual
Trigger ID:
3864784App:
next-js/15-app-router-todoApp directory:
apps/next-js/15-app-router-todoWorkbench branch:
wizard-ci-3864784-next-js-15-app-router-todoWizard branch:
mainExamples branch:
mainPostHog (MCP) branch:
masterTimestamp: 2026-01-21T22:24:45.649Z
Duration: 321.6s